Refactor #28
Refactor #28
Conversation
| operatorClient pboperator.OperatorClient | ||
| } | ||
|
|
||
| func NewGRPCDaemon( |
There was a problem hiding this comment.
newTdexClient more appropriate maybe ?
There was a problem hiding this comment.
I called it like this because it's possible on the daemon side to add other interfaces in the feature like for example HTTP, or JSON-RPC, etc.
There was a problem hiding this comment.
yeah its ok newTdexGrpcClient but still is not a daemon, but a client the data struct its returning
| _, message, err := k.conn.ReadMessage() | ||
| if err != nil { | ||
| if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) { | ||
| panic(err) |
There was a problem hiding this comment.
not great to panic here deep in code, better to lift up eventually and handle in cmd maybe?
There was a problem hiding this comment.
is this the fix for #24 ? shoudnt we connect by ourself again via code?
There was a problem hiding this comment.
this is specifically intended. After debugging and testing how this issue affects our service, basically what happens when Cloudflare closes the connection is that k.conn.ReadMessage() panics instead of returning the IsUnexpectedCloseError.
For this reason, we MUST recover the paniced code from the websocket pkg and signal that a reconnection attempt must be performed. In case the error is detected by this line (but I never experienced this so far) , we panic like websocket would be to preserve the behavior.
There was a problem hiding this comment.
maybe better to add some commented line to explain why things are done this way, what do you think?
| @@ -168,7 +216,7 @@ func (k *service) parseFeed(msg []byte) ports.PriceFeed { | |||
| return &priceFeed{ | |||
| market: mkt, | |||
| price: &price{ | |||
| basePrice: basePrice.String(), | |||
| basePrice: basePrice.StringFixed(8), | |||
There was a problem hiding this comment.
is it possible to have a float with 0.xxx here? there fore the stirng fixed shoudl be 10?
There was a problem hiding this comment.
only 3 decimals seems not enough to me. For BTC/USD ,for example, the base price (hence 1/(BTCUSD price) expresses a BTC unit, therefore 8 decimal precision.
There was a problem hiding this comment.
8 decimals plus the dot and the base unit, therefore StringFixed(10) ?
This contains breaking changes to move to a newer version of the feeder.
Config file
The biggest change from the user POV is the config file: the service can be now configured so that it allows updating prices of a market for multiple daemons (previously only one daemon could be handled). Of course, more than one market (and relative target daemons) can be defined, you can refer to the readme for more insights.
By default the service searches for a config.json file in the current path but it is still possible to define the full path of the file by exporting the env var FEEDER_CONFIG_PATH.
Internal structure
The very internal structure of the service has been completely revised and simplified. The application service is essentially an orchestrator of 2 main components (ports): TdexDaemon and PriceFeeder. Whenever a price feed related to some market is received, it is forwarded to the daemon(s) by calling the UpdateMarketPrice RPC.
For both interfaces a concrete implementation can be found in the infrastructure folder, respectively a gRPC impl of the daemon and a Kraken version of the feeder. Other impls can/will be added later easily just by implementing the service sticking with the proper interface.
Important to note that the kraken feeder bug (#24) has been fixed: the service now attempts to re-establish a socket connection with kraken in case it drops unexpectedly for any reason.
This closes #24.
Please @tiero review this.